Add dec!() macro by example
#692
Conversation
There was a problem hiding this comment.
Sorry for the delay in getting to this - and thank you for putting this together!
Added a couple of comments, however it really boils down to two requested changes I think:
First is to put this behind a feature flag macros. That way, we can remove some unnecessary ambiguity (also, making this an opt in feature). It means prelude will keep working until people have a chance to migrate over.
The other thing we should do is clean up the old macros folder (i.e. remove it completely). Personally, I think having an opinionated way forward is better - also, since it is a separate crate it should be fine to completely remove (i.e. folks can still use the old crate). Of course, this requires some clean up of the documentation to remove the reference to the crate too.
Thanks again!
| @@ -64,7 +64,7 @@ pub use maths::MathematicalOps; | |||
| pub mod prelude { | |||
| #[cfg(feature = "maths")] | |||
| pub use crate::maths::MathematicalOps; | |||
| pub use crate::{Decimal, RoundingStrategy}; | |||
| pub use crate::{dec, Decimal, RoundingStrategy}; | |||
There was a problem hiding this comment.
If we feature flag it, we'd just need to optionally include it here.
| /// `dec!(#![run_time] radix: my_radix, exp: my_exp, 10)` | ||
| #[macro_export] | ||
| macro_rules! dec { | ||
| (#![run_time] $($rest:tt)+) => { |
There was a problem hiding this comment.
Small nit: typically parameter attributes would not have the exclamation mark (e.g. #[run_time]. That said, I wonder if it's worthwhile just splitting this out completely - e.g. dec_debug!(xyz) or debug_dec!(xyz)
There was a problem hiding this comment.
This emulates an outer attribute, as though it had been written before the macro. Really making one that can be put outside would be a big effort. So I opted for putting it inside, which in Rust is written with an exclamation mark.
It's a little more than debug, also giving dynamic parameters, which is why I called it run_time.
For the feature gate, do you see that as only affecting the prelude, or the overall availability?
There was a problem hiding this comment.
Sorry - I may have been a little ambiguous! I realize I also skipped over the comment "Furthermore the expressions you pass to the options, which must normally be const, become dynamic" which kind of rules out the renaming of the macro (e.g. to dec_debug!).
Anyway, I wonder if we should remove the exclamation mark (!) so it matches the current parameter attribute style in Rust?
e.g.
dec!(#[run_time] radix: my_radix, exp: my_exp, 10);Subtle, but effectively it's just removing the exclamation mark in the run_time attribute. e.g. see here for an example: https://rust-lang.github.io/rfcs/2565-formal-function-parameter-attributes.html
Regarding the feature gate - I think yes for prelude for sure. Ideally it'd be the macro too (e.g. wrap in a mod with a conditional use perhaps?) however if it's too tricky then it's probably not that problematic to keep it.
As discussed, here is a cleaned up, further optimised and integrated new implementation of
dec!. It is a completelyconstand compile-time drop in replacement. This requires Rust 1.79.Your
Result<…String…>is not destructible, i.e. not usable inconst. Therefore the new parser has its ownResult, that can be transparently converted by?. This also required splittingtry_from_i128_with_scalewhich thus became simpler.For now the parser does only exact in
i128. Rounding should be addable, if you want to switch parsing completely to this.Being generic on a smaller type doesn’t currently seem possible in
const. Should a benchmark show this to be slower than parsing intou64, the old and new parsers might continue to coexist, till a solution is found.